Tests: Use UTC-based utcnow().date() instead of localtime-based date.today()
#441
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amotl
force-pushed
the
amo/fix-date-today
branch
5 times, most recently
from
ee86c14 to
07b8d79
Compare
Member
There was a problem hiding this comment.
Other than the test addition this looks good to me.
The commit message could be clarified a bit. Using UTC is not automatically better - it depends on the use case.
What is a problem is that we were comparing utc variants with local variants in
crate-python/src/crate/client/sqlalchemy/doctests/itests.txt
Lines 70 to 84 in a849cea
That is the problem and caused test flakyness.
src/crate/testing/test_datetime.py
Outdated
| import time_machine | ||
|
|
||
|
|
||
| class DateTodayTest(TestCase): |
Member
There was a problem hiding this comment.
Please remove, there's no need to create tests that verify documented behavior of the standard library.
Member
Author
There was a problem hiding this comment.
Thanks. I've removed the test cases and amended the commit message.
`datetime.date.today()` returns the current local date. In order to reduce test flakyness when comparing UTC-based datetimes with localtime- based variants, `datetime.utcnow().date()` is a better choice.
amotl
force-pushed
the
amo/fix-date-today
branch
from
07b8d79 to
4cf9d00
Compare
mfussenegger
approved these changes
Jul 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
utcnow()andutcfromtimestamp()for Python 3 #440, which went too far.About
datetime.date.today()returns the current local date. So, its outcome depends on the system time zone. Because we want to use UTC-based date and time objects instead,datetime.utcnow().date()is a better choice.